refactor(permission-controller): decouple permission middleware via messenger actions#8532
Open
rekmarks wants to merge 9 commits intofb/create-method-middlewarefrom
Open
refactor(permission-controller): decouple permission middleware via messenger actions#8532rekmarks wants to merge 9 commits intofb/create-method-middlewarefrom
rekmarks wants to merge 9 commits intofb/create-method-middlewarefrom
Conversation
Member
Author
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 706f8dc. Configure here.
FrederikBolding
force-pushed
the
fb/create-method-middleware
branch
from
fb74b9f to
a8f3aa6
Compare
This reverts commit 9efdcfe.
…essenger actions Replace the hook-based permission middleware factory with a standalone `createPermissionMiddleware` export that dispatches through messenger actions (`PermissionController:executeRestrictedMethod` and `PermissionController:hasUnrestrictedMethod`). Removes the `createPermissionMiddleware` property from `PermissionController`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Now that the permission middleware invokes restricted methods through the messenger, `getRestrictedMethod` has no remaining external consumers and is made `#`-private. Its caller signature is tightened so `requestingOrigin` is required, eliminating a dead optional-origin branch in `#getTypedPermissionSpecification`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add a `JsonRpcEngineV2` variant of the standalone permission middleware factory that uses the same messenger actions as the v1 factory. The existing `createPermissionMiddleware` is marked `@deprecated` in favor of the v2 variant. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
rekmarks
force-pushed
the
rekm/permission-methodmiddleware
branch
from
706f8dc to
f04c517
Compare
- Add v2 middleware tests for caveats, method-not-found, and undefined result - Add direct messenger action tests for hasUnrestrictedMethod and executeRestrictedMethod - Clarify changelog wording for standalone createPermissionMiddleware - Update ARCHITECTURE.md middleware example to use V2 and introduce the messenger - Clarify hasUnrestrictedMethod JSDoc about unknown methods
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Advances #4238
Reverts #8502
permission-middleware.tsas a standalonecreatePermissionMiddleware({ messenger, subject })factory that dispatches through thePermissionController:executeRestrictedMethodandPermissionController:hasUnrestrictedMethodmessenger actions instead of bound controller hooks. Removes thecreatePermissionMiddlewareproperty fromPermissionController.hasUnrestrictedMethodas a public method / messenger action, and makesgetRestrictedMethod#-private (it has no remaining external consumers now that the middleware goes through the messenger).undefined, the middleware now propagates the plainErrorthrown byexecuteRestrictedMethod; the JSON-RPC engine serializes it as a standard internal error response instead of a custominternalErrorwith arequestdata payload.Note
Medium Risk
Breaking API change removes
PermissionController.createPermissionMiddlewarein favor of standalone middleware factories and new messenger actions, so downstream integrations must be updated. Behavior around error propagation changes slightly (e.g., undefined restricted-method results now surface as generic internal errors), which could affect callers/tests.Overview
Decouples permission enforcement middleware from
PermissionController. The controller no longer exposescreatePermissionMiddleware; instead the package exports standalonecreatePermissionMiddleware(legacyJsonRpcEngine, now deprecated) andcreatePermissionMiddlewareV2(JsonRpcEngineV2) that route enforcement through messenger calls.Adds messenger-exposed APIs needed by the new middleware (
PermissionController:hasUnrestrictedMethodandPermissionController:executeRestrictedMethod), makesgetRestrictedMethodprivate, and updates tests/docs/changelog to reflect the new integration pattern and v2 engine support. Error handling is adjusted soundefinedrestricted-method results propagate as a plainError(serialized by the engine as a standard internal error) rather than a custom JSON-RPCinternalErrorpayload.Reviewed by Cursor Bugbot for commit 9eb56a6. Bugbot is set up for automated code reviews on this repo. Configure here.